Accessibility issues #80
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changes mobile nav trigger from a tag to button tag so that it can be triggered by keyboard. Adds additional aria-label and aria-controls to improve screenreader feedback
Add adjustments to mobile trigger styles introduced by replacing the a tag with a button tag
joelpittet
requested changes
Oct 28, 2024
| box-sizing: border-box !important; | ||
| margin-top: 7px !important; | ||
| padding: 11px 13px !important; | ||
| } |
There was a problem hiding this comment.
This appears to be all you need and matches the CLF specificity, assuming this file is loaded after the CLF
#ubc7-unit .navbar .btn-navbar {
height: 35px;
width: 45px;
padding: 11px 13px;
}
There was a problem hiding this comment.
Oh, strange. I'm going to have to take another look at why I went so hard on the specificity. I hate using important, so I must have had a reason (or so I hope). Thanks for taking a look and setting me straight on that.
There was a problem hiding this comment.
@occupant I added .navbar into your selector so I did make it more specific than your original, maybe that was it?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had an accessibility scan on EXL and the one very problematic finding was that the mobile navigation trigger wasn't functional when navigating via keyboard (keyboard access was likely only ever tested in a desktop context).
This isn't a Galactus-specific issue, it's a CLF-wide issue.
Changing the
atag to abuttonallows for correct focus and function of the trigger. While this could also be done with just aria, I think using theatag here is less correct; there is no link to another location, it's a trigger.The unfortunte consequence of changing the element is that the styles need to be updated as well. I've added these in the
clf.drupal.cssfile, but I'm sure it could have a better home.I've also added the
aria-controlsandaria-labelattributes to improve screenreader feedback on the two collapsible regions in the header.There's more to do, but this seems to be low-hanging fruit.